Conversation
drupol
left a comment
There was a problem hiding this comment.
Hello,
Thanks for your first PR.
I made some feedback, let me know if you need some help.
|
For such a security relevant package such as a vpn software, we should build from source if possible. This would also allow us to patch out quirks such as relying on /usr |
Your claims about salts are valid and correct! My earlier statement was based on an incorrect misunderstanding, and I've updated the comment accordingly. Another way to address quirks is to make changes directly in the NordVPN repository, which was my implicit assumption for the next release. I could update this to build from scratch, but I don’t see the added security benefit. All other Linux distributions download from |
You are right that most distributions fetch the However, the key issue here is trust and verifiability. At the moment, there's no reliable way to verify that the binaries provided on One of the strengths of Nix is its focus on reproducibility. Building from source allows us (most of the time) to produce reproducible outputs. This enables a verifiable 1-to-1 mapping between the source code and the resulting binaries, which significantly improves the security of the software supply chain. Fortunately, thanks to recent community efforts, we’re getting close to being able to build the client fully from source. That’s why I believe it’s worth pushing in that direction. |
Gotcha. A malicious attacker might somehow tamper with their binaries. Building from source is the secure way to go. Ok, will do, thanks! |
|
Modified the package to build from source instead of extracting the .deb file. Verified that core features function correctly. Thank you all for your time! |
6a79c37 to
30f70e6
Compare
|
I've reduced privileges by using a dedicated Additionally, the One more thing, Thanks again for the review! |
ruffsl
left a comment
There was a problem hiding this comment.
I've shifted to unstable for some unrelated KDE fixes, but cherry picked these latest commits. Appreciate the update using the added libxml2 package. Working well.
|
Changes:
|
ruffsl
left a comment
There was a problem hiding this comment.
As for the nix code, I've tested the latest changes to this package and module locally, and all's still building and running as expected.
|
@different-error , looks like v4.2.0 has released. I tried it locally, and the CLI seems to be working well just the same: diff --git a/pkgs/nordvpn/package.nix b/pkgs/nordvpn/package.nix
index 0038a2d..b9c68c7 100644
--- a/pkgs/nordvpn/package.nix
+++ b/pkgs/nordvpn/package.nix
@@ -42,13 +42,13 @@ let
in
buildGoModule (finalAttrs: {
pname = "nordvpn";
- version = "4.0.0";
+ version = "4.2.0";
src = fetchFromGitHub {
owner = "NordSecurity";
repo = "nordvpn-linux";
tag = finalAttrs.version;
- hash = "sha256-0GgMIFi6qrO7NG94vvWprwWr+8j87M5eH5W/pCtSWqs=";
+ hash = "sha256-9uh/UkOS84tVeW/d6qQ6bYPXzGXEoD21QHzrcMcdj7M=";
};
nativeBuildInputs = [
@@ -64,7 +64,7 @@ buildGoModule (finalAttrs: {
libxml2_13
];
- vendorHash = "sha256-GREoxjXqb28nabNvvcGjQA1rG4h9e/gINqEPY4++6Oo=";
+ vendorHash = "sha256-eUM69CQjbML8fWRG8H3w6x4M+E51YrXX/UCUFHerQmM=";
modPostBuild = ''
patch -p0 < ${./gokogiri-xpath-expression.patch}Perhaps we could also start a separate PR to begin packaging the new GUI that NordVPN open sourced with v4.2.0:
|
Thanks for the hashes.
Sounds good, I can (eventually..) start a draft PR or two for GUI and meshnet support. Looks like they use flutter and I can't find documentation for |
I got building and running fairly easily once I switched to using
I originally went down the wrabbit hole that is NordVPN's build/packaging pipeline for multi architectures/distros, where they use I think this patch could be made more idiomatic by either spinning out src or gui into separate nix files, so that it's still simple to use override stuff like nixpkgs/pkgs/by-name/mu/multipass/gui.nix Line 20 in c231242 |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-already-reviewed/2617/2601 |
|
hey, really looking forward to this being added in the near future... thanks to everyone who has been working on this |
|
What are the blockers/status of this project? Could a minimal viable version be shipped soon? |
pkgs/by-name/no/nordvpn/package.nix
Outdated
| # cgo build dependencies go here | ||
| # https://github.com/NixOS/nixpkgs/blob/master/doc/languages-frameworks/go.section.md#envcgo_enabled-var-go-cgo_enabled | ||
| # libxml2 2.14.[0-4] breaks daemon | ||
| libxml2_13 |
There was a problem hiding this comment.
Funny, with libxml2 being an original blocker when this PR was started, it looks like upstream has sense replaced it xsltproc instead in release v4.2.1 (with the current release now also being v4.2.2).
yeah it seems to have found itself lodged in "ready for review" for quite some time now.. in the meantime I intend to update it with latest changes from NordVPN sometime next week. thank you everyone for your patience. |
|
Any insight about why meshnet is not supported? I've been using the package just fine until I got hit with an error while trying to use meshnet and I could not find anything useful on the logs. |
@andersonjoseph , meshnet requires dependencies such as |
|
Just a quick version bump to 4.3.1. Going to work on the Flutter GUI now. Hopefully it turns out not too difficult thanks to the prior effort of @ruffsl |
|
Ok, I think the previous build passed because of some cache weirdness. Strangely when I had tested the binary, it spat out that the binary had used version 4.3.1. Anyway, latest commits use the correct vendor and src hashes for 4.3.1. |
|
Attempting to build nordvpn's flutter gui. You can find what I have so far here. It does not build atm, complains with the following error: Seems like I need to add |
|
I've added flutter gui support to the package and removed the salt. Also to avoid rebuilding the cli twice, I've separated out the package into cli.nix and gui.nix. I had to patch their linux CMakeLists.txt to use pkgconfig to find the correct x11 path. Tested and verified that both standalone package and module work as intended over openvpn and nordlynx.
I intend to work on incorporating meshnet next which I would start on Dec 26th. Hopefully not too difficult thanks to the prior efforts of @dimkNevidimk! |
|
meshnet progress update:
https://github.com/different-error/nixpkgs/tree/nordvpn-meshnet |
|
update:
Surprisingly, I don't see "meshnet" in the nordvpn settings. https://github.com/different-error/nixpkgs/tree/nordvpn-meshnet I've caught the flu. I intend to get back to this when I feel healthy enough to do so. |
|
Sorry for not replying earlier, but I think NordVPN team dropped meshnet support completely: |
They changed their mind and decided to keep it. |
|
I could use some help getting meshnet working. Please base your changes around my feature branch |
I will take a look when I get home from holiday travel (in a couple of hours) 👌 TL;DR: I got Meshnet working, but it tries to edit /etc/hosts, which causes permission errors. If anyone knows a clean way to grant write permissions to /etc/hosts, it would be great. Another solution is to send a patch with a --no-ns-hosts flag to the upstream repo so we can disable the write attempts and handle hostnames declaratively. |
|
Some nits. Tested successful connection using the GUI. While progress with meshnet continues, seeing that including it would cause this PR to increase significantly in size, I think we should PR the current changes without meshnet support and include meshnet in the next PR. I presently intend to break this PR into smaller, newer ones to facilitate review. |




Add the popular NordVPN to NixOS. Tested using the following configuration:
The configuration was tested by running:
There is another PR (#220616) for NordVPN, which is over two years old and has been stale for a year. Additionally, there are issues requesting NordVPN support for NixOS here and here.
I chose to extract the .deb package instead of building from source
to avoid modifying or leaking the salt. Meshnet is not yet supported, but core NordVPN features work. I’ll create another PR once the Meshnet issues are resolved.Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.